Enable integration tests for all services (fixes #4)#34
Enable integration tests for all services (fixes #4)#34marcosfelt wants to merge 3 commits intomainfrom
Conversation
This commit addresses issue #4 by: - Restoring the MoleculeResolverPatched class to mock _resilient_request - Uncommenting tests for all services: OPSIN, PubChem, CompTox, CTS, CAS, CIR, NIST - Enabling the OPSIN batchmode test - Creating responses.json file for caching test responses - Keeping Chemeo test commented (requires API key) - Keeping ChEBI test commented (benchmark data lacks chebi_id field) - Fixing method name from get_molecule_from_CAS_registryx to get_molecule_from_CAS_registry - Fixing test_opsin_batchmode to properly access result data Tests now run for all major services as requested in the issue. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@simonmb A quick attempt using claude code |
Added missing offline_status_codes parameter and updated type hints to match the actual MoleculeResolver._resilient_request signature. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@marcosfelt, thanks for this! For ChEBI, do we actually need the IDs? We could also search by name like with the others? Or am I missing something? |
Changed bare 'raise' to 'raise ValueError' with descriptive message when property count is not exactly 1. This fixes RuntimeError in tests. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@simonmb I'm guessing you're right. I haven't looked at CHEBI in over two years. On another note, these tests take a long time (30 minutes) - should we only run them on a schedule? |
|
Yeah, I saw they took this long. But why though? We have about 100 entries, or am I wrong? Using the online version actually getting the results from the services should be faster than what it's taking now by getting the results from a mock. I suspect something is off. Did you run this locally? |
|
We definitely need to add a call to search ethanol on every service online as well. I just noticed by chance that the CAS registry introduced an obligatory api key to their requests. Will work on that later. But having a test on online accessibility for one pretty well known component makes sense. |
|
I just realized 2 services weren't working. (I just fixed them) So we should add online searches to the tests, There were a lot of changes in the past 6 months on the services side... |
2 participants
This PR addresses issue #4 by enabling integration tests for all services.
Changes
Test Infrastructure
MoleculeResolverPatchedclass that mocks_resilient_requestto cache API responsesresponses.jsonfile for storing cached test responsesServices with Tests Enabled
Services Kept Commented
chebi_idfield)Fixes
get_molecule_from_CAS_registryxtoget_molecule_from_CAS_registrytest_opsin_batchmodeto properly accessr[0].SMILESinstead ofr.SMILESTesting
Tests will run automatically in GitHub Actions CI pipeline. The tests use cached responses to avoid hitting external APIs during CI runs.
Closes #4